feat: Support for GKE private clusters without default node pool #2408
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
@ciroclover, I think, in such an arrangement, it would never be accepted. First of all, you have to use autogen to generate all submodules automatically. Also, don't forget to generate docs and perform proper linting... I would recommend you make yourself familiar with the contribution guide |
ciroclover
force-pushed
the
private-cluster-node-pool
branch
from
390dcc1 to
d985e40
Compare
|
ciroclover
force-pushed
the
private-cluster-node-pool
branch
from
cbab4aa to
3800ac7
Compare
ciroclover
force-pushed
the
private-cluster-node-pool
branch
from
3800ac7 to
eeaf95d
Compare
|
Rebased with the main branch, fixed the template conflict, and ran |
|
/gcbrun |
|
Hi, I would also benefit from this PR. Is there anything that is stopping it from further review? |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for creating GKE clusters without a default node pool by making the node_pool block dynamic and adjusting related logic. The changes correctly handle the case where no node pools are defined. However, I've identified a critical pre-existing issue in how the inline default node pool is configured, which this PR touches. It incorrectly uses configuration from the first user-defined node pool (var.node_pools[0]), which can lead to misconfigurations. I've provided a detailed comment on this with recommendations for a safer approach. Additionally, I've suggested a simplification for computing the cluster name, which you also hinted at in your description.
|
/gcbrun |
4 participants
Fixes #2407
The diff shows a massive difference due to the "dynamic" block indentation.
The changes are:
initial_node_countwhen is not defined in the node-pool [ref].node_pools = [][ref]main.tf. I honestly believe that we could just usecluster_name_computed = var.name, however, more tests should be needed. [ref]